Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Skip member data by property #832

Merged
merged 1 commit into from
May 15, 2018
Merged

Skip member data by property #832

merged 1 commit into from
May 15, 2018

Conversation

Tratcher
Copy link
Member

@Tratcher Tratcher added this to the 2.2.0-mq milestone May 15, 2018
@Tratcher Tratcher self-assigned this May 15, 2018
@Tratcher Tratcher requested review from pakrym and JunTaoLuo May 15, 2018 18:19
@pakrym
Copy link
Contributor

pakrym commented May 15, 2018

I hate it a little bit. Can we do a source package or something else to carry common logic?

JunTaoLuo
JunTaoLuo approved these changes May 15, 2018
View reviewed changes
Copy link
Contributor

@JunTaoLuo JunTaoLuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I agree, would be nice to consolidate the logic. We discussed this when I made the duplication here and we thought it would be okay since we didn't expect this logic to ever change. Turns out that assumption did not hold for long.

@Tratcher Tratcher merged commit 782cfd3 into dev May 15, 2018
@Tratcher Tratcher deleted the feature/skip branch May 15, 2018 18:38
@Tratcher
Copy link
Member Author

Logged https://github.com/aspnet/Logging/issues/833 for all the duplication.

@Tratcher Tratcher mentioned this pull request Dec 13, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pakrym pakrym Awaiting requested review from pakrym

1 more reviewer

@JunTaoLuo JunTaoLuo JunTaoLuo approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@Tratcher Tratcher

Labels
None yet
Projects
None yet
Milestone
2.2.0-mq
Development

Successfully merging this pull request may close these issues.
3 participants
@Tratcher @pakrym @JunTaoLuo